Skip to content

feat(plugins): register_command(override=True) to shadow a built-in command - #50054

Open
arminanton wants to merge 2 commits into
NousResearch:mainfrom
arminanton:feat/plugin-register-command-override
Open

feat(plugins): register_command(override=True) to shadow a built-in command#50054
arminanton wants to merge 2 commits into
NousResearch:mainfrom
arminanton:feat/plugin-register-command-override

Conversation

@arminanton

@arminanton arminanton commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

Plugins can intentionally shadow a built-in slash command by passing override=True to PluginContext.register_command. The default remains unchanged: a conflicting command without that flag is rejected.

All three user dispatch paths consult one shared authorized override resolver before built-in handling. CLI, messaging gateway, and TUI therefore return the plugin result when an override is authorized.

Security

Third-party command overrides fail closed. A user or project plugin must receive the commands.override capability through plugins.entries.<plugin_id>.granted_capabilities, or use the deprecated allow_command_override: true compatibility setting. Missing consent, unreadable configuration, and ungranted plugins cannot register the override. Bundled plugins retain the existing trusted plugin treatment.

Compatibility

Normal plugin commands still run through the existing fallback path. Built-in conflicts still reject by default. The legacy allow_command_override setting maps to the capability registry so existing configuration can migrate without changing behavior.

Tests

The plugin suite covers default conflict rejection, denied registration, capability and legacy authorization, bundled plugin authorization, and the shared resolver contract.

Behavioral dispatch tests register an authorized override of the real /help built-in and invoke it through CLI, gateway, and TUI entry points. Each test proves that the plugin receives the raw arguments, its result reaches the caller, and built-in handling does not run. Inverse tests attempt the same registration without a grant, prove that registration fails, and then invoke /help to confirm built-in behavior remains active.

Validated with:

scripts/run_tests.sh tests/hermes_cli/test_plugins.py tests/cli/test_plugin_command_override.py tests/gateway/test_plugin_command_override.py tests/tui_gateway/test_plugin_command_override.py -q

Result: 76 tests passed.

venv/bin/python -m ruff check cli.py gateway/run.py hermes_cli/plugin_capabilities.py hermes_cli/plugins.py tests/hermes_cli/test_plugins.py tests/cli/test_plugin_command_override.py tests/gateway/test_plugin_command_override.py tests/tui_gateway/test_plugin_command_override.py tui_gateway/methods_tools.py

Result: all checks passed.

@alt-glitch alt-glitch added type/feature New feature or request comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have labels Jun 21, 2026
@arminanton arminanton mentioned this pull request Jun 21, 2026
Open
teknium1
teknium1 reviewed Jul 14, 2026
View reviewed changes

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for extending the plugin command surface. The collision is currently rejected on main (hermes_cli/plugins.py:560-569), but this implementation does not yet provide a safe, consistent override.

Problems

  • CLI reaches plugin handlers only in its post-built-in fallback (cli.py:8921-8985), and gateway does the same (gateway/run.py:10057-10072). TUI checks plugins first (tui_gateway/server.py:11892-11901), so override=True would behave differently by surface.
  • Current tool overrides require an explicit, fail-closed per-plugin operator opt-in (hermes_cli/plugins.py:409-470). The new command override has no equivalent authorization boundary.
  • The diff has no tests or docs update, while the current collision test is at tests/hermes_cli/test_plugins.py:1863-1872 and the documented API promises built-ins take precedence at website/docs/developer-guide/plugins/index.md:788-805.

Suggested changes

  • Centralize authorized override resolution across CLI, gateway, and TUI; add a fail-closed per-plugin gate and cross-surface regression tests; then update the documented API and precedence contract.

Automated hermes-sweeper review.

Comment thread hermes_cli/plugins.py
@@ -418,6 +418,7 @@ def register_command(
handler: Callable,
description: str = "",
args_hint: str = "",
override: bool = False,
) -> None:
"""Register a slash command (e.g. ``/lcm``) available in CLI and gateway sessions.

@teknium1 teknium1 Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

override=True only changes registration here. CLI and gateway dispatch built-ins before their plugin fallback (cli.py:8921-8985, gateway/run.py:10057-10072), while TUI checks plugins first (tui_gateway/server.py:11892-11901). Please add a shared, authorized resolution path and cross-surface tests before exposing this option.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 14, 2026
@arminanton
arminanton force-pushed the feat/plugin-register-command-override branch from 352b131 to 393642c Compare August 22, 2026 21:39
@arminanton

arminanton commented Aug 22, 2026

Copy link
Copy Markdown
Author

Rebased onto current main and made override behavior consistent + gated.

Consistent precedence across all three surfaces (sweeper): previously CLI/gateway resolved plugins only in the post-built-in fallback while TUI checked plugins first, so override=True behaved differently per surface. Added a shared get_plugin_command_override_handler(name) that returns a handler only for registrations flagged override_builtin=True, and each surface now short-circuits to it before built-in dispatch — CLI (cli.process_command, also covers the TUI slash-worker), gateway (gateway/run.py), and TUI (tui_gateway/methods_tools.py). Plain/unauthorized plugin commands still yield to built-ins everywhere.

Fail-closed operator opt-in (sweeper): command override is gated behind the same mechanism as tool override — a new commands.override capability (plugin_capabilities.py) with _command_override_allowed() mirroring _tool_override_allowed() (bundled = trusted; everyone else needs explicit consent/legacy key; any config-read failure = denied). register_command(override=True) without consent raises PluginCommandOverrideError and leaves the built-in intact.

Added a cross-surface precedence test + opt-in gate tests. test_plugins.py → 71 passed (172 across the plugin/command regression suites). Precedence contract documented in the plugins developer guide.

@arminanton
arminanton force-pushed the feat/plugin-register-command-override branch 2 times, most recently from 7e72003 to b80b40f Compare August 24, 2026 17:17
@arminanton
arminanton marked this pull request as ready for review August 24, 2026 17:18
@arminanton
arminanton force-pushed the feat/plugin-register-command-override branch from b80b40f to 7da8a25 Compare August 26, 2026 13:05
@arminanton
arminanton force-pushed the feat/plugin-register-command-override branch from 7da8a25 to 5831555 Compare August 26, 2026 14:32
This was referenced Aug 27, 2026
Open
Open
Open

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@teknium1 teknium1 teknium1 left review comments

Assignees

No one assigned

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/feature New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@arminanton @teknium1 @alt-glitch